fix(warehouses): Opralog: Use Entries.LogicallyDeleted column to remove duplicates#182
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughUpdates OAuth2 credentials for the Trino iceberg rest-catalogue, modifies data transformation models to include UTC timezone-aware timestamps and logical deletion filtering, and removes default environment variable values from the Trino DBT profile configuration. Changes
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @warehouses/accelerator/transform/accelerator/profiles.yml:
- Around line 8-14: The local env-local file is missing the DBT_TRINO_*
environment variables required by profiles.yml; add DBT_TRINO_USER,
DBT_TRINO_PASSWORD, DBT_TRINO_HTTP_SCHEME, DBT_TRINO_HOST, DBT_TRINO_PORT,
DBT_TRINO_CATALOG and DBT_TRINO_CATALOG_SCHEMA_PREFIX to infra/local/env-local
with values that match the Trino service in docker-compose.yml (e.g., service
host, port, HTTP scheme and local DB/catalog/schema prefixes) so profiles.yml
can successfully read user/password/http_scheme/host/port/database/schema via
env_var.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
infra/local/trino/etc/catalog/playground.propertieswarehouses/accelerator/transform/accelerator/models/staging/opralogweb/base/base_opralogweb__entries.sqlwarehouses/accelerator/transform/accelerator/models/staging/opralogweb/stg_opralogweb__mcr_equipment_downtime.sqlwarehouses/accelerator/transform/accelerator/profiles.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: elt-common end-to-end tests
🔇 Additional comments (7)
infra/local/trino/etc/catalog/playground.properties (1)
8-8: Credential update appears intentional.The OAuth2 credential change from
trino:s3cr3ttomachine-infra:s3cr3taligns with the PR's infrastructure adjustments. For local development, this appears appropriate.warehouses/accelerator/transform/accelerator/models/staging/opralogweb/base/base_opralogweb__entries.sql (3)
14-14: Good addition of explicit UTC timezone handling.Making the timestamp explicitly timezone-aware with UTC prevents ambiguity and ensures consistent temporal comparisons downstream.
16-16: Trimming whitespace improves data quality.Applying
trim()to theadditional_commentfield prevents leading/trailing whitespace issues in downstream processing.
17-20: Boolean conversion is correct and actively used downstream.The
CASEstatement properly converts thelogically_deletedstring flag ('Y' or other values) to a boolean. The downstream modelstg_opralogweb__mcr_equipment_downtimeconfirms this is being used to filter out deleted entries wherelogically_deleted = false.warehouses/accelerator/transform/accelerator/models/staging/opralogweb/stg_opralogweb__mcr_equipment_downtime.sql (3)
39-39: Core filter correctly excludes logically deleted entries.The
logically_deleted = falsecondition implements the PR objective by filtering out entries marked as deleted in the Opralog application. This prevents duplicate or logically-deleted records from appearing in the final transformation.
1-1: Clarify the purpose of the epoch filter.The addition of
fault_date >= from_iso8601_date({{ OPRALOG_EPOCH }})filters out entries before 25 April 2017. Whilst this may be intentional data quality improvement, it's not mentioned in the PR description which focuses on handling logically deleted entries.Is this epoch filter intentional for this PR, or should it be addressed separately?
Also applies to: 40-40
41-41: Logbook filter properly maintained.The
logbook_name = {{ MCR_LOGBOOK }}condition has been appropriately preserved and reordered for logical flow with the new filters.
1 participant
Summary
The Opralog application allows deleting entries but these are not removed from the database tables. Instead a flag is set to say that have been deleted. Use this flag to only pull non-deleted entries through to the final transformation.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.